fix(goosefs): decouple OpenDAL root from URL path to avoid ObjectStore cache-key collision#7612
Merged
yanghua merged 3 commits intoJul 6, 2026
Conversation
…e cache key collision Previously GooseFsStoreProvider baked the URL path into the OpenDAL Operator root and also into the ObjectStoreRegistry cache prefix. That made two datasets under the same master (e.g. goosefs://host:9200/a.lance and goosefs://host:9200/b.lance) collide in the registry cache: the second open reused the first Operator whose root still pointed at the first dataset, so queries on B silently returned A's data. Rework the provider to follow the S3-style model already used by tos/oss: - Operator root is cluster-wide (default "/", overridable via goosefs_root storage option or GOOSEFS_ROOT env var), never per-URL. - extract_path returns the URL path (percent-decoded) as the per-request object key, so a single shared Operator can route requests to the correct dataset. - calculate_object_store_prefix keys the registry on scheme+authority (plus the custom root when non-default), so datasets under the same master intentionally share one Operator without cross-contamination. Add regression tests covering: - extract_path for basic, root, deep and percent-encoded URLs; - prefix sharing across datasets on the same master; - prefix isolation across masters; - prefix inclusion of a custom goosefs_root; - master-address resolution from URL, default port and storage options; - root resolution defaults and overrides.
1e76390 to
0c59f54
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
yanghua
reviewed
Jul 6, 2026
yanghua
left a comment
Collaborator
There was a problem hiding this comment.
Left two comments, otherwise LGTM.
Comment on lines
198
to
205
| fn extract_path(&self, url: &Url) -> Result<Path> { | ||
| // `Path::from_url_path` percent-decodes the input and normalizes | ||
| // separators, matching how `S3StoreProvider` (default trait impl) | ||
| // treats bucket-relative keys. | ||
| Path::from_url_path(url.path()).map_err(|e| { | ||
| Error::invalid_input(format!("Invalid path in URL '{}': {}", url.path(), e)) | ||
| }) | ||
| } |
Collaborator
There was a problem hiding this comment.
It is duplicated with providers.rs, can we reuse it?
| /// datasets under the same master can share a single cached Operator. | ||
| /// | ||
| /// Priority: | ||
| /// 1. `storage_options["goosefs_root"]` |
Collaborator
There was a problem hiding this comment.
Can we explain this config at the top of the file?
…g docs Address review feedback: - Remove GooseFsStoreProvider::extract_path override; the trait default in ObjectStoreProvider is byte-for-byte identical (Path::from_url_path with the same error mapping), so overriding it is pure duplication. Behavior is unchanged and the existing extract_path unit tests still pass by resolving to the trait default. - Move all supported config keys (goosefs_master_addr / goosefs_root / goosefs_write_type / goosefs_block_size / goosefs_chunk_size / goosefs_auth_type / goosefs_auth_username) into a single table at the top of the file, alongside the existing path-handling model. This puts the goosefs_root semantics next to the rest of the configuration surface instead of hiding them on a private helper. The per-function doc on resolve_root is trimmed to a pointer at the file-level docs. - Drop the now-unused object_store::path::Path import.
CI runs rustdoc with -D warnings, which upgraded a broken intra-doc
link into a hard error:
error: unresolved link to `ObjectStoreRegistry`
--> rust/lance-io/src/object_store/providers/goosefs.rs:55:46
`ObjectStoreRegistry` lives in the top-level `lance` crate and is not
in scope from `lance-io`, so rustdoc cannot resolve the [`...`] link
syntax. `Operator` is re-exported from `opendal` and is likewise not a
local item, so the same syntax would trip the next rustdoc run.
Downgrade both references from intra-doc links to plain code spans
(backticks only). They are still rendered as inline code but are no
longer resolved as links, which matches how the surrounding docs
already refer to `ObjectStoreRegistry`. The other bracketed links in
this file (`Operator`-free ones like `ObjectStore`,
`ObjectStoreProvider::extract_path`, `GooseFsStoreProvider`,
`Self::extract_path`) all resolve to items `use`d in this module and
remain untouched.
Verified locally with:
RUSTDOCFLAGS="-D warnings" cargo doc -p lance-io --features goosefs --no-deps
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The GooseFS
ObjectStoreprovider used to bake the URL path into OpenDAL'srootand always return an emptyextract_path. BecauseObjectStoreRegistrykeys its cache on the provider prefix (
goosefs$host:port), two differentdataset URLs under the same master resolved to the same cached
ObjectStore— with arootlocked to whichever URL opened it first. Thesecond open reused that store and, since
extract_pathwas empty, the readwent to the first dataset's bytes and schema.
This PR aligns the GooseFS provider with the S3/GCS/OSS model.
Reproducer (before this fix)
Root cause
rust/lance-io/src/object_store/providers/goosefs.rshad three coupledchoices that together broke the registry cache invariants:
new_storesetroot = base_path.path()— a per-URL root.extract_pathunconditionally returnedPath::from(""), "trusting"the root to fully identify the object.
calculate_object_store_prefixreturned onlygoosefs$host:port,so the URL path did not participate in the cache key.
ObjectStoreRegistry::get_store(rust/lance-io/src/object_store/providers.rs)keys
active_storeson(prefix, params)and holdsWeak<ObjectStore>.As long as any live
Arc<ObjectStore>fora.lanceexists, openingb.lanceis a cache hit that returns the store rooted ata.lance,and the empty key from
extract_pathreads the wrong dataset.Fix (S3-style path model)
rootis cluster-wide, not per-URL. Defaults to/, overridablevia storage option
goosefs_rootor env varGOOSEFS_ROOT. Alldatasets under the same master share a single OpenDAL
Operator.extract_pathreturns the URL path (percent-decoded viaPath::from_url_path) as the per-request object key. OpenDAL'sbuild_rooted_abs_path(root, key)reconstructs the absolute GooseFSpath exactly as before.
calculate_object_store_prefixstays scoped togoosefs$host:portin the default case, and folds a custom root in as
goosefs$host:port#/rootso different roots remain isolated in the cache.
Net effect:
a.lanceandb.lanceshare one cachedOperator(thesame reuse benefit S3 already gets), and correctness comes from the
distinct object keys on each request.
Semantic parity with other providers
This mirrors
aws.rs/gcp.rs/oss.rs, which use the default traitimplementations of
extract_pathandcalculate_object_store_prefix.Tests
Added focused regression tests in the provider (all passing):
test_prefix_shared_across_datasets_same_master— same master, twodataset URLs share one cache prefix but yield distinct object
keys. This is the direct counter-assertion to the bug.
test_prefix_isolated_across_masters— different masters must notshare a cache entry.
test_prefix_includes_custom_root— a customgoosefs_rootisreflected in the prefix.
test_goosefs_extract_path_percent_decoded— percent-encoded segments(e.g.
%20) are decoded exactly once, guarding against double-encodingwhen the object store client re-encodes for transport.
Verification
cargo fmt -p lance-io -- --check→ cleancargo clippy -p lance-io --features goosefs --all-targets -- -D warnings→ 0 warningscargo test -p lance-io --features goosefs --lib→ 197 passed / 0 failedCompatibility
URL path becoming the OpenDAL root need no change — the same absolute
path is still resolved via
root="/"+ relative key.goosefs_root/GOOSEFS_ROOTfor tenants that wantto sandbox an
Operatorto a subtree. Different values get isolatedcache entries.